Skip to content

fix(extract): default the AST cache location to CWD, not the analyzed source tree (#1774)#1802

Closed
SimiSips wants to merge 1 commit into
Graphify-Labs:v8from
SimiSips:fix/extract-cache-root-default-1774
Closed

fix(extract): default the AST cache location to CWD, not the analyzed source tree (#1774)#1802
SimiSips wants to merge 1 commit into
Graphify-Labs:v8from
SimiSips:fix/extract-cache-root-default-1774

Conversation

@SimiSips

Copy link
Copy Markdown
Contributor

Fixes #1774.

Problem

extract() called without cache_root derives the cache root from the inferred common parent of the input files — i.e. the analyzed source tree. Analyzing a corpus outside CWD (a read-only knowledge base, someone else's checkout) silently creates graphify-out/cache/ inside that corpus, defeating output isolation via CWD. Reproduced on current v8 (0.9.13). The CLI and watcher are unaffected — they already pass cache_root explicitly (added in 01fb51b for #350) — so this only bites library callers and python -m graphify.extract.

Fix

One line: effective_root (the cache location) now defaults to Path('.') when cache_root is None, matching cache_dir()'s own default. An explicit cache_root still wins.

Deliberately not the patch suggested in the issue (defaulting root itself to CWD): root also anchors node-id relativization (path.relative_to(root)) and _augment_symbol_resolution_edges(), and pointing it at CWD would degrade ids for corpora outside CWD. Only the cache side changes; id/symbol anchoring is untouched.

Notes

  • file_hash() keys include the path relative to the cache root, so existing source-tree caches of library callers go cold once and re-extract (CLI/watch caches are unaffected). No correctness impact.
  • Cache entries for out-of-root files keep absolute source_file paths — an already-handled pass-through in _relativize_source_files_in / _absolutize_source_files_in.

Tests

  • New tests/test_extract_cache_location.py: default cache lands in CWD and the corpus stays untouched; the CWD cache round-trips on a second run; explicit cache_root still wins.
  • Full suite: uv run pytest tests/ -q3162 passed, 3 skipped on macOS (Python 3.12).

… source tree (Graphify-Labs#1774)

extract() without cache_root derived the cache root from the inferred
common parent of the input files, so analyzing a corpus outside CWD
silently created graphify-out/cache/ inside the analyzed source tree
(read-only corpora, other people's checkouts). The cache is an output:
effective_root now defaults to Path('.'), matching cache_dir()'s own
default. The inferred root still anchors node ids and symbol resolution
unchanged, and an explicit cache_root still wins (CLI/watch behavior is
untouched — they already pass it).
@safishamsi

Copy link
Copy Markdown
Collaborator

Thanks @SimiSips — you had the right diagnosis and the right default (the cache is an output; it belongs at CWD, not inside the analyzed tree). Landed in c3a42a3 (ships in 0.9.14), crediting you, and I kept your CWD-default and the two location tests.

I extended it because effective_root was doing three jobs behind one name: cache-dir location, file_hash key anchoring, and the XAML/C# project-scan boundary. Pointing that single value at CWD fixes the location but, for a corpus that lives outside CWD, makes file_hash fall back to absolute keys (so shared/CI cache reuse silently breaks) and makes the XAML .cs scan run over CWD instead of the corpus. So I split the two concepts: load_cached/save_cached take a new cache_root for where the dir lives, while root (the inferred common parent) still anchors keys, node ids and the XAML boundary. The parallel worker tuple carries both.

Net: your behavior for the in-CWD case is preserved, and out-of-CWD corpora keep portable relative keys. I added a regression test asserting keys stay relative for a corpus outside CWD (the property the one-liner would have lost). Also mirrors the location/anchor split that #1747 introduced for the stat index. Appreciate the fix.

@safishamsi safishamsi closed this Jul 13, 2026
safishamsi added a commit that referenced this pull request Jul 13, 2026
…1774)

With no explicit cache_root, extract() wrote graphify-out/cache/ under the
inferred common parent of the inputs — the analyzed source tree — so
scanning a read-only/foreign corpus silently polluted it.

The naive fix (point the root at CWD) breaks two other things that shared
the same parameter: file_hash keys become absolute/non-portable for an
out-of-CWD corpus, and the XAML/C# project-scan boundary would scan CWD
instead of the corpus. So split cache LOCATION from key/id ANCHOR:
load_cached/save_cached gain a cache_root arg for where the dir lives,
while `root` (inferred common parent) still anchors file_hash keys,
source_file relativization, node ids, and the XAML boundary. extract()
now locates the cache at CWD (or cache_root) but anchors on `root`; the
parallel worker tuple carries both. Existing callers passing cache_root
(CLI, watcher) are unchanged.

Adopts @SimiSips's #1802 (the CWD default + the two location tests) and
adds the decoupling plus a regression test that keys stay relative for a
corpus outside CWD — the property the one-line version would have lost.

Co-Authored-By: SimiSips <SimiSips@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extract() writes AST cache into the analyzed source tree, bypassing CWD (pollutes read-only corpora)

2 participants